[GlobalISel] Fix fewerElementsVectorPhi to insert after G_PHIs #87927
Conversation
|
@llvm/pr-subscribers-llvm-globalisel Author: Dávid Ferenc Szabó (dfszabo) ChangesCurrently the inserted mergelike instructions will be inserted at the location of the G_PHI. Seems like the behaviour was correct before, but the rework done in https://reviews.llvm.org/D114198 forgot to include the part which makes sure the instructions will be inserted after all the G_PHIs. Full diff: https://github.com/llvm/llvm-project/pull/87927.diff 1 Files Affected:
diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index 95c6a359e52ec6..9412fa85cd3856 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -4265,6 +4265,10 @@ LegalizerHelper::fewerElementsVectorPhi(GenericMachineInstr &MI,
}
}
+ // Set the insert point after the existing PHIs
+ MachineBasicBlock &MBB = *MI.getParent();
+ MIRBuilder.setInsertPt(MBB, MBB.getFirstNonPHI());
+
// Merge small outputs into MI's def.
if (NumLeftovers) {
mergeMixedSubvectors(MI.getReg(0), OutputRegs);
|
|
This issue came up with our downstream backend, but the problematic examples are passing with AArch64. Not easy to create a test for this, but on the other hand the change is fairly self evident. |
There is a failing unit test. Maybe that can give an idea of how this changes things. |
Yes indeed, the test fix show now nicely the expected behavior. |
Currently the inserted mergelike instructions will be inserted at the location of the G_PHI. Seems like the behaviour was correct before, but the rework done in https://reviews.llvm.org/D114198 forgot to include the part which makes sure the instructions will be inserted after all the G_PHIs.
|
Ping? |
…87927) Currently the inserted mergelike instructions will be inserted at the location of the G_PHI. Seems like the behaviour was correct before, but the rework done in https://reviews.llvm.org/D114198 forgot to include the part which makes sure the instructions will be inserted after all the G_PHIs.
|
/cherry-pick 2347020 |
Error: Command failed due to missing milestone. |
…87927) Currently the inserted mergelike instructions will be inserted at the location of the G_PHI. Seems like the behaviour was correct before, but the rework done in https://reviews.llvm.org/D114198 forgot to include the part which makes sure the instructions will be inserted after all the G_PHIs.
…87927) Currently the inserted mergelike instructions will be inserted at the location of the G_PHI. Seems like the behaviour was correct before, but the rework done in https://reviews.llvm.org/D114198 forgot to include the part which makes sure the instructions will be inserted after all the G_PHIs. (cherry picked from commit 2347020)
…87927) Currently the inserted mergelike instructions will be inserted at the location of the G_PHI. Seems like the behaviour was correct before, but the rework done in https://reviews.llvm.org/D114198 forgot to include the part which makes sure the instructions will be inserted after all the G_PHIs. (cherry picked from commit 2347020)
…87927) Currently the inserted mergelike instructions will be inserted at the location of the G_PHI. Seems like the behaviour was correct before, but the rework done in https://reviews.llvm.org/D114198 forgot to include the part which makes sure the instructions will be inserted after all the G_PHIs. (cherry picked from commit 2347020)
Currently the inserted mergelike instructions will be inserted at the location of the G_PHI. Seems like the behaviour was correct before, but the rework done in https://reviews.llvm.org/D114198 forgot to include the part which makes sure the instructions will be inserted after all the G_PHIs.